Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Codecov Uploader instead of Bash uploader #19

Merged
merged 17 commits into from
Jun 7, 2022

Conversation

paymand
Copy link
Contributor

@paymand paymand commented May 23, 2022

Support for the Bash Uploader, as you can see on the page, is deprecated on February 1st, 2022. Even though it still seems to work fine, I think it will eventually be sunset at some point.

The idea is to use Codecov Uploader.

Also see the blog post below for more info:
https://about.codecov.io/blog/introducing-codecovs-new-uploader/

I have updated the tests and they pass:

➜  codecov-buildkite-plugin git:(codecov_uploader) ✗ docker-compose run --rm tests
Creating codecov-buildkite-plugin_tests_run ... done
 ✓ Post-command succeeds
 ✓ Post-command succeeds with arguments
 ✓ Post-command succeeds with -Z
 ✓ Post-command is skipped if command failed and skip_on_fail=true
 ✓ Pre-exit succeeds

5 tests, 0 failures

I haven't actually tested the upload, but I will try to test it on a branch.

@joscha
Copy link
Owner

joscha commented May 23, 2022

Great! It's about time to replace the bahs based uploader - can you make sure that the plugin still works as expected on a variety of hosts? The bash uploader had very few dependencies. I haven't looked at your PR in detail but I saw an APK call somewhere for example, which we can't easily make a prerequisite possibly.

@paymand
Copy link
Contributor Author

paymand commented May 24, 2022

I saw an APK call somewhere for example, which we can't easily make a prerequisite possibly.

Added support for MacOS and other Linux flavors here: 97431ea

@paymand
Copy link
Contributor Author

paymand commented May 24, 2022

can you make sure that the plugin still works as expected on a variety of hosts?

Will test this and let you know.

@paymand
Copy link
Contributor Author

paymand commented May 24, 2022

@joscha works for me on our buildkite pipeline. Tested it with one of our projects and the coverage files are uploaded correctly. Feel free to give it a spin if you have the possibility.

Copy link
Owner

@joscha joscha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this! I haven't been able to test it as I don't currently have a codecov project available to me.

@@ -47,6 +176,9 @@ main() {
local ci_env
ci_env=$(bash <(curl -s -S --connect-timeout 10 --retry 3 --retry-delay 10 https://codecov.io/env))

codecov_command=./codecov
get_codecov_uploader
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means that on each run a new uploader is downloaded; always latest currently. I wonder if we can allow a fixed version set by the plugin config and then cache away that particular version. Similar to what is done here: https://github.com/joscha/sauce-connect-buildkite-plugin/blob/master/hooks/pre-command#L30

That way the uploader stays stable longer and does not rely on the network fetch each time nor will cause trouble (read massive amounts of requests) when there are many many builds run from one subnet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 388c47d
I have added two new configs: uploader_version and tmp_dir with proper defaults.
The downloaded binary will be cached accordingly.

@@ -25,6 +25,135 @@ plugin_read_list_into_result() {
[[ ${#result[@]} -gt 0 ]] || return 1
}


get_os() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the uploader is only available for plain Linux, alpine and Macos (and windows which we don't currently support) can we simplify this one? If it just returned the strings linux, alpine or macos we can feed that directly into the download URL

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 388c47d

local os=$(get_os)
local os_array=($os)
if [ -n ${os_array[1]} ] && [ ${os_array[1]} = "Alpine" ]; then
apk add gnupg
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does an alpine allow this install on the fly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, in my test it seems to be working:

➜  codecov-buildkite-plugin git:(codecov_uploader) docker run -it alpine:3.14   
/ # gpg
/bin/sh: gpg: not found
/ # apk add gnupg
(1/28) Installing libgpg-error (1.42-r0)
(2/28) Installing libassuan (2.5.5-r0)
(3/28) Installing libcap (2.50-r0)
(4/28) Installing libffi (3.3-r2)
(5/28) Installing libintl (0.21-r0)
(6/28) Installing libblkid (2.37.4-r0)
(7/28) Installing libmount (2.37.4-r0)
(8/28) Installing pcre (8.44-r0)
(9/28) Installing glib (2.68.3-r0)
(10/28) Installing ncurses-terminfo-base (6.2_p20210612-r0)
(11/28) Installing ncurses-libs (6.2_p20210612-r0)
(12/28) Installing libgcrypt (1.9.4-r0)
(13/28) Installing libsecret (0.20.4-r1)
(14/28) Installing pinentry (1.1.1-r0)
Executing pinentry-1.1.1-r0.post-install
(15/28) Installing libbz2 (1.0.8-r1)
(16/28) Installing gmp (6.2.1-r1)
(17/28) Installing nettle (3.7.3-r0)
(18/28) Installing p11-kit (0.23.22-r0)
(19/28) Installing libtasn1 (4.17.0-r0)
(20/28) Installing libunistring (0.9.10-r1)
(21/28) Installing gnutls (3.7.1-r0)
(22/28) Installing libksba (1.5.1-r0)
(23/28) Installing gdbm (1.19-r0)
(24/28) Installing libsasl (2.1.28-r0)
(25/28) Installing libldap (2.4.58-r0)
(26/28) Installing npth (1.6-r0)
(27/28) Installing sqlite-libs (3.35.5-r0)
(28/28) Installing gnupg (2.2.31-r0)
Executing busybox-1.33.1-r7.trigger
OK: 25 MiB in 42 packages
/ # gpg --version
gpg (GnuPG) 2.2.31
libgcrypt 1.9.4
Copyright (C) 2021 Free Software Foundation, Inc.
License GNU GPL-3.0-or-later <https://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Home: /root/.gnupg
Supported algorithms:
Pubkey: RSA, ELG, DSA, ECDH, ECDSA, EDDSA
Cipher: IDEA, 3DES, CAST5, BLOWFISH, AES, AES192, AES256, TWOFISH,
        CAMELLIA128, CAMELLIA192, CAMELLIA256
Hash: SHA1, RIPEMD160, SHA256, SHA384, SHA512, SHA224
Compression: Uncompressed, ZIP, ZLIB, BZIP2

curl -Os https://uploader.codecov.io/latest/linux/codecov
curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM
curl -Os https://uploader.codecov.io/latest/linux/codecov.SHA256SUM.sig
gpgv codecov.SHA256SUM.sig codecov.SHA256SUM
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes gpg a prereq for the plugin. We should name it in the plugin.yml

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 388c47d

[[ ${#result[@]} -gt 0 ]] || return 1
}
script_path="$( cd -- "$(dirname "$0")" >/dev/null 2>&1 ; pwd -P )"
source "${script_path}/../lib/utils.bash"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, this fails on CI:

/var/lib/buildkite-agent/plugins/github-com-peakon-codecov-buildkite-plugin-uploader/hooks/post-command: line 5: /tmp/../lib/utils.bash: No such file or directory

@joscha any idea how to do this? I moved the common code to another file, because I needed to use some of the code in the tests...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I remember solving this before. I can't find a code reference right now, but I remember two ways:

set a variable in your script where you want to include the file:

REPO_DIR="$(git rev-parse --show-toplevel)"

which will always give you the plugin root (as it is a valid checkout) and then include the file via:

source "${REPO_DIR}/lib/utils.bash"

in your bats test you can then just stub out the call to git to return the right path:

REPO_DIR="${BATS_TEST_DIRNAME}/../.."

setup() {
  stub git "rev-parse --show-toplevel : echo ${REPO_DIR}"
}

Copy link
Contributor Author

@paymand paymand May 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm, I did try the second suggestion, but git rev-parse --show-toplevel gets the path to the root of our checked out project on the agent and tries to include lib/utils.bash which obviously doesn't exist there and it fails. I think I'll go with the first solution: bringing back the extracted code to post-command and stubbing the required code in the tests.

@paymand paymand requested a review from joscha May 25, 2022 17:08
plugin.yml Show resolved Hide resolved
@paymand
Copy link
Contributor Author

paymand commented May 27, 2022

@joscha would you mind doing another review? I have tested on CI and it works fine. Also tried with uploader_version: v0.2.3 and tmp_dir: /tmp/foo as you can see below:

gpg: key ED779869: public key "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>" imported
gpg: Total number processed: 1
gpg:               imported: 1  (RSA: 1)
/tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov
https://uploader.codecov.io/v0.2.3/linux/codecov
/tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov.SHA256SUM
https://uploader.codecov.io/v0.2.3/linux/codecov.SHA256SUM
/tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov.SHA256SUM.sig
https://uploader.codecov.io/v0.2.3/linux/codecov.SHA256SUM.sig
gpgv: Signature made Tue 17 May 2022 01:25:03 PM UTC using RSA key ID ED779869
gpgv: Good signature from "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>"
codecov: OK
[2022-05-27T11:33:45.384Z] ['info'] 
     _____          _
    / ____|        | |
   | |     ___   __| | ___  ___ _____   __
   | |    / _ \ / _` |/ _ \/ __/ _ \ \ / /
   | |___| (_) | (_| |  __/ (_| (_) \ V /
    \_____\___/ \__,_|\___|\___\___/ \_/

  Codecov report uploader 0.2.3

@paymand paymand requested a review from joscha May 27, 2022 20:07
@joscha
Copy link
Owner

joscha commented Jun 2, 2022

Hi @paymand, thanks for all the updates. Looking pretty good now! If I pushed a few changes to the bash code, would you be able to run your test on the combined code by any chance?

@paymand
Copy link
Contributor Author

paymand commented Jun 3, 2022

@joscha yes, I can. Please push your changes and I'll test it.

@joscha
Copy link
Owner

joscha commented Jun 3, 2022

@joscha yes, I can. Please push your changes and I'll test it.

Pushed them here: peakon#6 - mainly shellcheck, a few other minor changes

do
local local_path="${TMP_DIR}/${file}"
if [[ ! -e "${local_path}" ]]; then
>&2 echo "Local path will be: ${local_path}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>&2 echo "Local path will be: ${local_path}"
debug "Local path will be: ${local_path}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 827e857.

if [[ ! -e "${local_path}" ]]; then
>&2 echo "Local path will be: ${local_path}"
local remote_source="https://uploader.codecov.io/${CODECOV_VERSION}/${OS}/${file}"
>&2 echo "Source is: ${remote_source}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
>&2 echo "Source is: ${remote_source}"
debug "Source is: ${remote_source}"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 827e857.

@paymand
Copy link
Contributor Author

paymand commented Jun 7, 2022

@joscha Got it tested and it works fine:

gpg: key ED779869: public key "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>" imported
gpg: Total number processed: 1
gpg:               imported: 1  (RSA: 1)
Local path will be: /tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov
Source is: https://uploader.codecov.io/v0.2.3/linux/codecov
Local path will be: /tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov.SHA256SUM
Source is: https://uploader.codecov.io/v0.2.3/linux/codecov.SHA256SUM
Local path will be: /tmp/foo/codecov-buildkite-plugin/linux/v0.2.3/codecov.SHA256SUM.sig
Source is: https://uploader.codecov.io/v0.2.3/linux/codecov.SHA256SUM.sig
gpgv: Signature made Tue 17 May 2022 01:25:03 PM UTC using RSA key ID ED779869
gpgv: Good signature from "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>"
codecov: OK
Unable to find image 'buildpack-deps:jessie-scm' locally
jessie-scm: Pulling from library/buildpack-deps
Digest: sha256:f1d9fcc8ad402af3b095cdec22a9dd6f0dbba630eab7b0f032c842ecafdaceb9
Status: Downloaded newer image for buildpack-deps:jessie-scm
[2022-06-07T07:12:37.912Z] ['info'] 
     _____          _
    / ____|        | |
   | |     ___   __| | ___  ___ _____   __
   | |    / _ \ / _` |/ _ \/ __/ _ \ \ / /
   | |___| (_) | (_| |  __/ (_| (_) \ V /
    \_____\___/ \__,_|\___|\___\___/ \_/

  Codecov report uploader 0.2.3

@joscha joscha merged commit 3af774b into joscha:master Jun 7, 2022
@joscha
Copy link
Owner

joscha commented Jun 7, 2022

Nice work!

@paymand
Copy link
Contributor Author

paymand commented Jun 7, 2022

Also tested without uploader_version and tmp_dir:

gpg: key ED779869: "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>" not changed
gpg: Total number processed: 1
gpg:              unchanged: 1
Local path will be: /tmp/codecov-buildkite-plugin/linux/latest/codecov
Source is: https://uploader.codecov.io/latest/linux/codecov
Local path will be: /tmp/codecov-buildkite-plugin/linux/latest/codecov.SHA256SUM
Source is: https://uploader.codecov.io/latest/linux/codecov.SHA256SUM
Local path will be: /tmp/codecov-buildkite-plugin/linux/latest/codecov.SHA256SUM.sig
Source is: https://uploader.codecov.io/latest/linux/codecov.SHA256SUM.sig
gpgv: Signature made Tue 17 May 2022 01:25:03 PM UTC using RSA key ID ED779869
gpgv: Good signature from "Codecov Uploader (Codecov Uploader Verification Key) <security@codecov.io>"
codecov: OK
[2022-06-07T07:27:48.563Z] ['info'] 
     _____          _
    / ____|        | |
   | |     ___   __| | ___  ___ _____   __
   | |    / _ \ / _` |/ _ \/ __/ _ \ \ / /
   | |___| (_) | (_| |  __/ (_| (_) \ V /
    \_____\___/ \__,_|\___|\___\___/ \_/

  Codecov report uploader 0.2.3

Good to merge IMHO 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants